Fix tsconfig.json and Lint errors #290

derekvmcintire · 2025-01-09T21:39:38Z

Summary of changes

Updated tsconfig.json - left comments on each line to document the purpose of each configuration
Fixed type and lint errors where I could find them including:
- import order
- file extensions in imports
- vitest methods
- explicit type imports
- typecasting where necessary
- declared a type module for heat-stack/types/vite-env-only.d.ts - not sure if there is a better solution for this, but it works

Unfixable Type Errors

There were unfixable errors in heat-stack/app/utils/providers/github.server.ts which i typecasted to unknown before typecasting again to the needed type, but I suspect there could be issues with either the data or the type here that need to be addressed further. I left @TODOs in the code - but it might be better to create an issue if there isn't a straight forward answer here.

Lint Errors Not Fixed

There are still some lint errors from a React hook being used in a test outside of a component (it is possible the use() method here is intended to be imported from somewhere else?)

Screenshots

derekvmcintire · 2025-01-09T22:05:03Z

heat-stack/app/utils/providers/github.server.ts

 		return new GitHubStrategy(
 			{
 				clientID: process.env.GITHUB_CLIENT_ID,
 				clientSecret: process.env.GITHUB_CLIENT_SECRET,
 				callbackURL: '/auth/github/callback',
-			},
+			} as unknown as GitHubStrategyOptions, // @TODO fix types here, clientID and callbackURL do not exist on GitHubStrategyOptions


@todo here because clientID and callbackURL do not exist on type GitHubStrategyOptions, so we either need to figure out what type we should be using, or potentially update the code here

derekvmcintire · 2025-01-09T22:05:38Z

heat-stack/app/utils/providers/github.server.ts

@@ -25,20 +26,20 @@ const shouldMock =
 	process.env.NODE_ENV === 'test'

 export class GitHubProvider implements AuthProvider {
-	getAuthStrategy() {
+	getAuthStrategy(): Strategy<ProviderUser, any> { // @TODO double check the types here


added @todo here for someone to double check the types

derekvmcintire · 2025-01-15T15:11:56Z

Updates 1/15/2025

This should be working now. Here is what I discovered:

The giant list of errors was from the new changes that had just been merged in (Thad, you might have pointed this out but it didn't sink in last night). So I fetched and merged the new changes and was able to reproduce locally.
The new typescript configuration requires explicit file extensions in imports, and they were missing from the new changes. I fixed them, and then re-configured my VSCode to add file extensions when using auto imports (details below).
The error we were seeing from the ../build/... directory was not caused by typescript looking at the build directory itself, but it is actually a dynamic import in server/index.tsx: await import('../build/server/index.js'). There was already a tsignore comment above it: // @ts-expect-error - the file might not exist yet but it will but I had accidentally added a space between @ and t so it wasn't working.

More Details

To configure VSCode to add file extensions automatically

Go to your settings.json file: cmd + shift + p, then select “Open User Settings (JSON)
Add the following (or replace existing configurations):

"typescript.preferences.importModuleSpecifier": "relative",  // or "auto" for dynamic imports
"typescript.preferences.importModuleSpecifierEnding": "minimal",  // "auto" if possible

Going forward, you will see typescript errors for imports without file extensions

This is because of our "module": "NodeNext" and "moduleResolution": "nodenext" settings in tsconfig.json. Below is a quick overview of why we probably want this setting, but if we feel like this is something we don't want to do I can try to work out a different ts configuration.

Requiring file extensions:

Pros:
- Aligns with the current Node.js and ES Module specification, and makes our project more compatible with modern JavaScript standards.
- Helps avoid ambiguity when resolving files, especially when working across different platforms.
- More consistent and predictable behavior, especially when moving code between environments.
Cons:
- Requires us to be more explicit with imports.
- It’s an extra thing to manage.

Not requiring file extensions (i.e., trying to configure TypeScript to automatically infer extensions):

Pros:
- The import syntax feels cleaner and more concise.
- Matches the behavior we might be accustomed to in other TypeScript or JavaScript projects.
Cons:
- Potential compatibility issues with certain tools or libraries that rely on explicit file extensions.
- Can lead to bugs when files are not resolved correctly.

plocket

I'm not familiar with this code, but I've been told it's good to go and I should merge (and I'm at a computer)

commit 2d09a76 Author: Derek McIntire <[email protected]> Date: Tue Jan 21 19:44:27 2025 -0500 Fix tsconfig.json and Lint errors (#290) * fixed all ts errors and lint warnings * Add comments to tsconfig * Don't override includes * add stuff * change target to ES2020 * add exclude list * remove commented out lines * More explicit exclude * try include as well as exclude * Letting claude give it a try * I dont think build is a hidden directory * undo claude * Type check working now * fix eslint comment as well * Add noUncheckedIndexAccess commit ab29024 Author: Derek McIntire <[email protected]> Date: Tue Jan 14 20:42:54 2025 -0500 223 heatload graph (#288) * Use rules engine output to populate heat load graph * update graph record type * run prettier for select files * fix dataKeys * Use correct data and make chart pretty * Graph working, but needs clean up * Clean up and comments * Comments on calculation functions * Add icon, still need to add values underneath chart * restore ws changes to types * Add columns at bottom of chart * Fix reversed average and max in grid * Fix responsiveness issues * Extract utility and data building functions, tool tip and constants * Add comment with link to external calculation docs * Fix tool tip * quick refactor * clean up * refactor calculating min and max for Y axis * set start and end temps once * Add 2f to x axis so tool tip is easier to invoke * review changes to HeatLoad.tsx * All issues except legend and tool type type errors addressed * legend working with valid value for layout * break out legend and fix avg heat load endpoint * Adjust x axis ticks and make design set point a global constant * fix HeatLoadGraphToolTip type error * Remove hard coded data * Add comment to calculateAvgHeatLoadEndPoint * comment out inclusion_override * Fix type errors * re-comment tsconfig commit 98edbe0 Author: AdamFinkle <[email protected]> Date: Mon Dec 16 15:22:38 2024 -0500 Fixed errors that CI workflow caught (#285)

derekvmcintire added 3 commits December 21, 2024 18:01

fixed all ts errors and lint warnings

7337558

Add comments to tsconfig

3be08f1

Don't override includes

0227873

derekvmcintire commented Jan 9, 2025

View reviewed changes

derekvmcintire added 11 commits January 14, 2025 20:55

add stuff

e30499f

change target to ES2020

20d36e9

add exclude list

7761e69

remove commented out lines

2d3224f

More explicit exclude

7762161

try include as well as exclude

cb18d3e

Letting claude give it a try

b0664c9

I dont think build is a hidden directory

6b138e8

undo claude

c8deaa0

Merge remote-tracking branch 'origin/main' into ts_config_updates

f597432

Type check working now

8a39b10

derekvmcintire added 2 commits January 15, 2025 10:13

fix eslint comment as well

4c8c303

Add noUncheckedIndexAccess

13fc701

plocket approved these changes Jan 22, 2025

View reviewed changes

plocket merged commit 2d09a76 into codeforboston:main Jan 22, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tsconfig.json and Lint errors #290

Fix tsconfig.json and Lint errors #290

derekvmcintire commented Jan 9, 2025 •

edited

Loading

derekvmcintire Jan 9, 2025

derekvmcintire Jan 9, 2025

derekvmcintire commented Jan 15, 2025

plocket left a comment

Fix tsconfig.json and Lint errors #290

Fix tsconfig.json and Lint errors #290

Conversation

derekvmcintire commented Jan 9, 2025 • edited Loading

Summary of changes

Unfixable Type Errors

Lint Errors Not Fixed

Screenshots

derekvmcintire Jan 9, 2025

Choose a reason for hiding this comment

derekvmcintire Jan 9, 2025

Choose a reason for hiding this comment

derekvmcintire commented Jan 15, 2025

Updates 1/15/2025

This should be working now. Here is what I discovered:

More Details

To configure VSCode to add file extensions automatically

Going forward, you will see typescript errors for imports without file extensions

plocket left a comment

Choose a reason for hiding this comment

derekvmcintire commented Jan 9, 2025 •

edited

Loading